Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(SDK-312) Add option --parallel to pdk test unit #154

Merged
merged 1 commit into from
Aug 2, 2017

Conversation

austb
Copy link
Contributor

@austb austb commented Jul 14, 2017

Waiting for a new version of puppetlabs_spec_helper for the updated rake task

Update:

I now believe it's waiting for a new version of puppet-module-gems

Update 2:

v0.1.0 of puppet-module-gems has been released

# TODO: test selection
[File.join(PDK::Util.module_root, 'bin', 'rake'), 'spec']
if opts.key?(:parallel)
[File.join(PDK::Util.module_root, 'bin', 'rake'), 'parallel']
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need to change once puppetlabs_spec_helper is updated. Added the updated parallel_spec task to the module's Rakefile as parallel to avoid name conflicts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is just deciding between two rake tasks, you can remove a bit of duplication here by changing the method content to something like this

rake_task = opts.key?(:parallel) ? 'parallel' : 'spec'
[File.join(PDK::Util.module_root, 'bin', 'rake'), rake_task]

@austb austb force-pushed the parallel_tests branch 3 times, most recently from ff054ae to b1c9fe6 Compare July 20, 2017 20:22
@austb austb changed the title {WIP} Parallel tests {WIP} (SDK-312) Add option --parallel to pdk test unit Jul 21, 2017
@austb austb added the feature label Jul 21, 2017
@austb austb changed the title {WIP} (SDK-312) Add option --parallel to pdk test unit (SDK-312) Add option --parallel to pdk test unit Aug 1, 2017
@austb austb force-pushed the parallel_tests branch 2 times, most recently from 044208b to 11b9407 Compare August 1, 2017 18:15
@austb austb requested review from DavidS and rodjek August 1, 2017 23:38
@austb austb force-pushed the parallel_tests branch 3 times, most recently from 37819ff to 0e8d00f Compare August 1, 2017 23:53
@puppetlabs puppetlabs deleted a comment from coveralls Aug 2, 2017
@puppetlabs puppetlabs deleted a comment from coveralls Aug 2, 2017
@puppetlabs puppetlabs deleted a comment from coveralls Aug 2, 2017
@puppetlabs puppetlabs deleted a comment from coveralls Aug 2, 2017
@puppetlabs puppetlabs deleted a comment from coveralls Aug 2, 2017
@puppetlabs puppetlabs deleted a comment from coveralls Aug 2, 2017
Copy link
Contributor

@rodjek rodjek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of recommended changes to the spec descriptions, otherwise 👍. Nice work @austb

require 'pdk/tests/unit'

describe PDK::Test::Unit do
describe 'merging json results' do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should change the description of this example group to the method being tested describe '.merge_json_results'

end
end

describe '#cmd' do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a class method, so .cmd would be better.

end
end

describe '#parse_output' do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a class method, so .parse_output would be better.


# Allow any_instance stubbing of Commands
# rubocop:disable RSpec/AnyInstance
describe '#list' do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a class method, so .list would be better.

end
end

describe '#invoke' do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a class method, so .invoke would be better.

Uses the puppetlabs_spec_helper Rake task 'parallel_spec' to run spec
tests in parallel. This will run the Rake task in multiple Processes
to avoid the limitation on Thread speed increases due to MRI Ruby's
Global Interpreter Lock. This means that rspec initialization effort
will be duplicated.
@coveralls
Copy link

Coverage Status

Coverage increased (+3.7%) to 90.219% when pulling 8e39016 on austb:parallel_tests into 9cb598e on puppetlabs:master.

@puppetlabs puppetlabs deleted a comment from coveralls Aug 2, 2017
@austb austb merged commit 48741a5 into puppetlabs:master Aug 2, 2017
@austb austb deleted the parallel_tests branch August 2, 2017 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants